Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

NoMethodError when nodes are coerced more then once in a request #194

Closed
wants to merge 2 commits into from
Closed

NoMethodError when nodes are coerced more then once in a request #194

wants to merge 2 commits into from

Conversation

mrGrazy
Copy link

@mrGrazy mrGrazy commented Jan 16, 2019

We've bumped into a bug with rich text coercion in instances where a request includes multiple links to the same entity, for example:

We have a page with a reference to a textBlock which has a rich text field linking back to the same instance of page.

This throws an error:

NoMethodError (undefined method `[]' for <Contentful::Page[page] id='...'>:Contentful::Page):
contentful (2.11.0) lib/contentful/entry.rb:96:in `method_missing'
contentful (2.11.0) lib/contentful/coercions.rb:153:in `coerce_link'
contentful (2.11.0) lib/contentful/coercions.rb:128:in `block in coerce_block'
contentful (2.11.0) lib/contentful/coercions.rb:126:in `each'
contentful (2.11.0) lib/contentful/coercions.rb:126:in `each_with_index'
contentful (2.11.0) lib/contentful/coercions.rb:126:in `coerce_block'
contentful (2.11.0) lib/contentful/coercions.rb:136:in `block in coerce_block'
contentful (2.11.0) lib/contentful/coercions.rb:126:in `each'
contentful (2.11.0) lib/contentful/coercions.rb:126:in `each_with_index'
contentful (2.11.0) lib/contentful/coercions.rb:126:in `coerce_block'
contentful (2.11.0) lib/contentful/coercions.rb:106:in `coerce'
...

This is because when the link in the rich text is coerced, the ['target'] has already been converted to a Page model and so the guard clause fails.

I believe this pull request fixes the issue.

I would like to add a test for this case with this PR, but I don't know how as an external contributor I can add new VCR cassettes without access to the contentful space that is used for testing. Happy to add if you can tell me how 😄

@mrGrazy mrGrazy changed the title Squash bug when nodes are visited coerced more then once in a request NoMethodError when nodes are coerced more then once in a request Jan 16, 2019
@dlitvakb
Copy link
Contributor

Thanks @mrGrazy,

I'll create the test case myself and push it.

Cheers

@mrGrazy
Copy link
Author

mrGrazy commented Jan 21, 2019

Thanks @dlitvakb
Let me know if you need more details to reproduce it.

@mrGrazy mrGrazy closed this Jan 21, 2019
@mrGrazy mrGrazy reopened this Jan 21, 2019
@dlitvakb
Copy link
Contributor

Hey @mrGrazy,

Sorry for the delay, was looking for a few spare moments to get this fix in.

Will add this test case now.

Cheers

@dlitvakb dlitvakb mentioned this pull request Jan 23, 2019
Merged
@dlitvakb
Copy link
Contributor

This has now been merged and released as version 2.11.1.

Cheers

@mrGrazy
Copy link
Author

mrGrazy commented Jan 23, 2019

Thanks @dlitvakb 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mrGrazy @dlitvakb